Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for Yarn workspaces (PoC for #189) #200

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add support for Yarn workspaces (PoC for #189) #200

wants to merge 2 commits into from

Conversation

stephank
Copy link

@stephank stephank commented Aug 19, 2020 •
edited
Loading

This is a proof of concept for #189 (comment). Suggest we try keep discussion here just about implementation.

The idea is to run node2nix for each package individually, but point --yarn-workspace at the workspace top-level package.json so it can discover interdependencies which use custom derivations as src.

This code works but is crude, and only does part of the job. It generates a node-packages.nix like:

# This file has been generated by node2nix 1.8.0. Do not edit!

{nodeEnv, fetchurl, fetchgit, globalBuildInputs ? [], _at_foobar_slash_a}:

let
  sources = {
    # [...]
    "@foobar/a-../a" = {
      name = "_at_foobar_slash_a";
      packageName = "@foobar/a";
      version = "0.0.0";
      src = _at_foobar_slash_a;
    };
    # [...]
  };
  args = {
    name = "_at_foobar_slash_b";
    packageName = "@foobar/b";
    version = "0.0.0";
    src = ./.;
    dependencies = [
      # [...]
      sources."@foobar/a-../a"
      # [...]
    ];
    buildInputs = globalBuildInputs;
    meta = {
    };
    production = true;
    bypassCache = true;
    reconstructLock = true;
  };
in
{
  args = args;
  sources = sources;
  tarball = nodeEnv.buildNodeSourceDist args;
  package = nodeEnv.buildNodePackage args;
  shell = nodeEnv.buildNodeShell args;
  nodeDependencies = nodeEnv.buildNodeDependencies args;
}

Note the extra function parameters at the top, and the src attributes for workspace dependencies.

Some notes:

  • Doesn't support --lock. Doesn't make sense, because npm has no workspaces support. It would make sense if we had yarn.lock support. Either way, should probably make this error?
  • Doesn't yet update CompositionExpression. I haven't yet thought about what this should look like.
  • Also supports workspace: protocol with path (specified by Yarn), e.g.: workspace:../a (Using --yarn-workspace essentially injects those for you behind the scenes.)
  • Doesn't yet support workspace: protocol with version specifier (specified by Yarn), e.g.: workspace:^1.2.3
  • Doesn't support CollectionExpression. Doesn't make sense, I think? Should probably error?
  • Now that I'm writing this, I realize it'll always bundle all workspace dependencies, even if the package doesn't use them. Probably need to take another look at this, but still can discuss the overall idea. 🙃

@stephank
Copy link
Author

stephank commented Aug 19, 2020 •
edited
Loading

I've fixed some of the issues from the original attempt:

  • Now doesn't install workspace dependencies that weren't referenced by the subpackage being built. This new approach adds a deploymentConfig.packageOverrides to pick workspace dependencies from on-demand.
  • Can now be run against the workspace package.json, which is treated like any other package, but includes virtual dependencies on the subpackages. The subpackages also install devDependencies if !production in this case. Useful for repo-wide builds.
    • Don't think bundling the subpackage is the right approach here, because this should actually create relative symlinks in node_modules.

@stephank
Copy link
Author

stephank commented Aug 21, 2020 •
edited
Loading

I've extended the implementation to create symlinks when running node2nix on the workspace package.json. The WorkspaceSource now has a symlink flag, and these sources generate Nix with a string symlink attribute instead of a src attribute. In node-env.nix, this only touches on composePackage. (Sorry for the misc changes there, may undo it later.)

Some new obstacles:

  • It looks like npm linkStuff may be failing, but I'm not sure. I have a project with an sqlite3 dependency, and it can't find node-pre-gyp, while sqlite3 does specify it as a dependency.

  • In general, npm doesn't understand this setup anymore, and for example allowing npm install to run trashes the node_modules directory. So dontNpmInstall is required here.

  • I'm not sure if the node-shell setup can be made to work at all. The symlinks would have to point to the original source tree where the user invoked Nix, but that's an impurity, I think? On top of that, subpackages may have specific dependencies that need to be available only in that subtree.

@svanderburg
Copy link
Owner

Hi,

Looks like a great first attempt. It seems that you have looked carefully at the structure and followed all my conventions, including using "old fashioned JavaScript" conventions.

I have to admit that I have not used Yarn workspaces myself, so in order to help you I need to make myself a bit more familiar with it.

A question about this feature: when you look at node2nix from a very high level perspective -- it is essentially a tool that runs npm install in a derivation and "tricks" NPM to not download any dependencies. by providing the required dependencies with Nix instead. So basically running NPM is a key aspect of what node2nix does.

Is Yarn workspaces also a feature that is supported by NPM? Or do you need to switch to yarn to get these installed? Running yarn in a derivation is not something node2nix can currently facilitate.

@stephank
Copy link
Author

stephank commented Aug 22, 2020 •
edited
Loading

Yarn workspaces are not supported by npm, no. So some of the obstacles are to be expected. The idea to try Yarn workspaces was to see if it'd highlight similar issues to what we might encounter with other monorepo setups. (Plus I have a bunch of projects using Yarn workspaces myself.)

Maybe I'm overthinking goals for #189, though? (No worries, btw. This is all just exploration.)

@jtojnar
Copy link
Contributor

jtojnar commented Oct 31, 2020

Looks like workspaces are supported by npm 7: https://github.com/npm/cli/blob/eedf93e1526f5834d549b68dab9258b6283921de/CHANGELOG.md#some-high-level-changes-and-improvements

@svanderburg
Copy link
Owner

@jtojnar That's corect, so now there's also a reason to add that feature.

Maybe you actually want to wait for my node-env.nix improvements -- this probably makes it a lot easier to implement it.

@basvandijk
Copy link

I'm trying to package the web/ui npm workspace of prometheus-2.31.0 and I would assume I need workspace support in node2nix for this. Any progress on this feature?

Also are there ways of working around the npm workspace by building the sub-packages individually but sharing the workspace's package-lock.json?

@teto teto mentioned this pull request Mar 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stephank @svanderburg @jtojnar @basvandijk